-
Notifications
You must be signed in to change notification settings - Fork 249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open folders with user altered filenames in SpikeGLX #1608
base: master
Are you sure you want to change the base?
Open folders with user altered filenames in SpikeGLX #1608
Conversation
Co-authored-by: Alessio Buccino <alejoe9187@gmail.com>
OK, I added the tests and this should be ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comments to hopefully make the reviewing easier.
@@ -82,14 +82,11 @@ class SpikeGLXRawIO(BaseRawWithBufferApiIO): | |||
|
|||
Notes | |||
----- | |||
* Contrary to other implementations this IO reads the entire folder and subfolders and: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this because it is not clear what other implementations this is referring to, probably the ones before but then I don't think it should be here.
* For imec device both "ap" and "lf" are extracted so one device have several "streams" | ||
* There are several versions depending the neuropixel probe generation (`1.x`/`2.x`/`3.x`) | ||
* Here, we assume that the `meta` file has the same structure across all generations. | ||
* This IO is developed based on neuropixel generation 2.0, single shank recordings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a note of the first version but we have gradually added support. This supports the shanked version just fine and also NHP and other varieties.
* Here, we assume that the `meta` file has the same structure across all generations. | ||
* This IO is developed based on neuropixel generation 2.0, single shank recordings. | ||
* This IO reads the entire folder and subfolders locating the `.bin` and `.meta` files | ||
* Handles gates and triggers as segments (based on the `_gt0`, `_gt1`, `_t0` , `_t1` in filenames) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added specific mention to gates and triggers as that is more in line with SpikeGLX documentation.
|
||
order_key = list({make_key(info) for info in info_list}) | ||
order_key = sorted(order_key) | ||
# This sets non-integers values before integers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this section is the core of the PR. It uses the same mechanism that we have for gates and triggers before but also for probe index.:
- It creates a set of unique combinations of (gate, trigger) and (probe, port,dock).
- That combination is used to create the index (segment index and probe index)
- The parameters of info are used to assign the segment and probe index with a dict.
The normalize lambda unifies this behavior. Maybe there is a better naming but I could not come with anything, it just follows the convention that Sam had before.
|
||
# Define stream base on device [imec|nidq], device_index and stream_kind [ap|lf] for imec | ||
for info in info_list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This defines the stream here after all the information is available. Previously, it was done inside the extract_info_function
.
@@ -488,13 +513,15 @@ def extract_stream_info(meta_file, meta): | |||
else: | |||
# NIDQ case | |||
has_sync_trace = False | |||
fname = Path(meta_file).stem | |||
|
|||
bin_file_path = meta["fileName"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the part that uses the meta["fileName"] (the original one) instead of the file_path in the system (which the user might have changed).
@@ -563,12 +593,15 @@ def extract_stream_info(meta_file, meta): | |||
info["trigger_num"] = trigger_num | |||
info["device"] = device | |||
info["stream_kind"] = stream_kind | |||
info["stream_name"] = stream_name | |||
info["device_kind"] = meta.get("typeThis", device.split(".")[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get the device kind directly from the metadata. imec or nidq. All non-production probes should have this in their metadata. For production probes this would rely on file parsing as we were doing before:
Should fix #1606
To solve #1606 this does two things:
Both things should make the reader more robust.
I am preparing some data for gin where the filename was changed. I will change this from draft once I add that.